Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Filter Admin Only Cards for Home Page #651

Merged
merged 1 commit into from
Oct 8, 2024
Merged

Filter Admin Only Cards for Home Page #651

merged 1 commit into from
Oct 8, 2024

Conversation

andrew032011
Copy link
Collaborator

@andrew032011 andrew032011 commented Oct 6, 2024 •
edited
Loading

Summary

We previously were only filtering navigation cards on the "Forms" tab, but not the home page. This is because we had two navigation card components, NavCard and NavigationCard, the first of which did not contain logic to filter against the "adminOnly" flag. We should allow adminOnly to be used on the homepage if we want to gate something through a feature flag.

Also, update the Homepage to use NavigationCard and delete NavCard. This consequently also fixes an alignment issue on the homepage too.

Notion/Figma Link

Test Plan

Verification steps:

  1. Check that the "Admin" and "Forms" tab still look the same
  2. Check that the homepage styling still looks acceptable (in terms of spacing and alignment of the cards)

Before:
Screenshot 2024-10-06 at 12 46 27 AM

After:
Screenshot 2024-10-06 at 12 46 21 AM

Notes

Breaking Changes

@andrew032011 andrew032011 requested a review from a team as a code owner October 6, 2024 04:47
@dti-github-bot
Copy link
Member

[diff-counting] Significant lines: 76.

@andrew032011 andrew032011 mentioned this pull request Oct 6, 2024
Open
kevinmram
kevinmram approved these changes Oct 6, 2024
View reviewed changes
Copy link
Contributor

@kevinmram kevinmram left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a good catch that we were using NavCard and NavigationCard- using just NavigationCard across both the homepage and other sections, reducing redundancy and simplifying our codebase. However, there are a few CSS changes, especially concerning responsive design. I'd double-checking that the adjustments maintain consistency in mobile views, particularly for the .card and .quick containers, but everything should be lookin' good! (you're the goat A.C.)

@andrew032011
Copy link
Collaborator Author

It's a good catch that we were using NavCard and NavigationCard- using just NavigationCard across both the homepage and other sections, reducing redundancy and simplifying our codebase. However, there are a few CSS changes, especially concerning responsive design. I'd double-checking that the adjustments maintain consistency in mobile views, particularly for the .card and .quick containers, but everything should be lookin' good! (you're the goat A.C.)

Thanks for the reminder! I checked this and it looks like the cards were previously not aligned, and now they are. So net-positive!

Before:

image

After:
image

@andrew032011 andrew032011 merged commit 4d161a9 into main Oct 8, 2024
17 checks passed
@andrew032011 andrew032011 deleted the axc/filter_admin_only_items_homepage branch October 8, 2024 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kevinmram kevinmram kevinmram approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@andrew032011 @dti-github-bot @kevinmram